Use `tray_icon` crate for better tray icon support #288

dastansam · 2024-08-12T04:47:56Z

closes #222

tested on MacOS

Screen.Recording.2024-08-12.at.10.49.15.mov

nazar-pc

Thanks for converting it! Didn't know you were working on it, ping me in such cases so we don't do the same work twice (I didn't start on this yet, don't worry).

Looks like a much nicer library than betrayer, wondering why it wasn't used in the first place 🤔

This will need to be tested on Windows and Linux, you'll also have to update documentation on required dependencies since it seems like we have some added in here. For Linux we should be using libayatana-appindicator3-dev as a dependency.

One thing that will likely make this unusable/unacceptable for Linux is that it is based on GTK3, not GTK4.
GTK3 is legacy and we don't compile/include it on Windows or macOS.
I think it'll work for Windows and macOS since it is not GTK3 that is used there to drive tray icon (hence the question why atk was installed on macOS).
On Linux https://github.com/iovxw/ksni was suggested in tauri-apps/tray-icon#107 and probably worth exploring (betrayer for example doesn't work on my system, but ksni seems to support more protocols and should start working).

Assuming we go with 2 libraries, the tray icon creation should probably be wrapped into a platform-specific struct that will handle the initialization and that we can store in the App instance.

Overall this is a good start, but needs more work and more testing before we can merge it.

P.S. Tray icons are a mess on Linux:

Cargo.toml

nazar-pc · 2024-08-12T05:00:53Z

src/frontend.rs

+        let image = image::load_from_memory(ICON)
+            .expect("Statically correct image; qed")
+            .to_rgba8();
+
+        let (width, height) = image.dimensions();


These variables will be unused on Windows and cause clippy warnings. They should be grouped with tray icon construction using let tray_icon = {};.

Also you can use image::load_from_memory_with_format(ICON, image::ImageFormat::Png) as more concrete method and you can use image.width() instead of extracting separate variables first.

src/frontend.rs

nazar-pc · 2024-08-12T05:06:54Z

src/frontend.rs

+            .with_menu(std::boxed::Box::new(
+                Menu::with_items(&[
+                    &MenuItem::with_id(
+                        TRAY_ICON_MENU_OPEN_ID,


Looks like you don't really need to assign ID explicitly, just create menu item with ::new() and then store ID in a variable with menu_item.id() call.

tray icon initialization and receiving menu events are in different scopes with the latest refactoring. do you think I should just save the IDs and expose a method to retrieve them?

I do. I basically expect something like platform-specific constructors or even functions that take Sender as an argument and return opaque TrayIcon instance back that frontend will need to keep around for tray icon to continue functioning.

nazar-pc · 2024-08-12T05:07:28Z

src/frontend.rs

+                Menu::with_items(&[
+                    &MenuItem::with_id(
+                        TRAY_ICON_MENU_OPEN_ID,
+                        T.tray_icon_open().to_string(),


Shoudn't need an allocation, try this:

Suggested change

T.tray_icon_open().to_string(),

&T.tray_icon_open(),

tried this, but it raises a compile error due to trait bound of AsrRef<str>

It needs to be de-referenced first: &*T.tray_icon_open()

src/frontend.rs

.github/workflows/rust.yml

nazar-pc

This is a step in the right direction, but a bit overengineered and Linux support is not implemented fully

.github/workflows/rust.yml

Cargo.toml

src/frontend.rs

nazar-pc · 2024-08-15T05:50:32Z

src/frontend.rs

+                Menu::with_items(&[
+                    &MenuItem::with_id(
+                        TRAY_ICON_MENU_OPEN_ID,
+                        T.tray_icon_open().to_string(),


It needs to be de-referenced first: &*T.tray_icon_open()

src/frontend.rs

dastansam · 2024-08-26T12:44:46Z

I haven't yet tested this on non-MacOS Linux machine but I think I resolved all the comments code-wise @nazar-pc

nazar-pc · 2024-08-30T22:03:08Z

I did some refactoring, fixed some issues and added single left click handling. Tested on Windows and Ubuntu and confirmed that it works there at least.

@dastansam can you confirm it still works on macOS as expected?

nazar-pc · 2024-08-30T22:04:32Z

Still doesn't work on my Linux due to lack of legacy icons support in the library, but it is not worse than before, so not a blocker.

dastansam · 2024-09-01T20:35:25Z

I did some refactoring, fixed some issues and added single left click handling. Tested on Windows and Ubuntu and confirmed that it works there at least.

@dastansam can you confirm it still works on macOS as expected?

open button works fine, but close button quits the application. I thought it would just hide the window. Maybe then we can rename it Quit?

nazar-pc · 2024-09-01T20:39:19Z

I thought it would just hide the window. Maybe then we can rename it Quit?

It closes the app. It was working like this until you changed it, so I restored it back to what it was. Renaming is possible, but we'll have to update a bunch of translations in that case. I'll take care of it tomorrow alongside with one more improvement.

Use tray_icon crate for better tray icon support

3657441

dastansam requested a review from nazar-pc August 12, 2024 04:50

nazar-pc mentioned this pull request Aug 12, 2024

macOS: Tray icon related issues & more #183

Closed

dastansam mentioned this pull request Aug 12, 2024

Install atk in github actions #289

Closed

Install atk in github actions

61f9f75

nazar-pc reviewed Aug 12, 2024

View reviewed changes

dastansam force-pushed the replace-tray-icon branch 2 times, most recently from 2209bac to fbb7b7c Compare August 14, 2024 17:33

Use different crate for Linux and platform generic tray icon struct

56d7e33

dastansam force-pushed the replace-tray-icon branch from fbb7b7c to 56d7e33 Compare August 14, 2024 18:52

dastansam requested a review from nazar-pc August 14, 2024 19:28

nazar-pc requested changes Aug 15, 2024

View reviewed changes

Introduce abstract, platform generic tray icon

acea905

dastansam force-pushed the replace-tray-icon branch from 33ff0a4 to acea905 Compare August 26, 2024 12:40

dastansam requested a review from nazar-pc August 26, 2024 12:43

Remove unnecessary GTK3 dependency from CI workflow

5786058

nazar-pc approved these changes Aug 30, 2024

View reviewed changes

nazar-pc added 8 commits August 31, 2024 01:15

Move tray icon implementations into submodules with no other changes

aa808d5

Minor refactoring

08642f8

Refactor tray icon implementation to a simple spawn function

0c3d641

Update tray-icon dependency

4697b05

Fix compilation dependencies on Linux

145d83f

Fix exit mapped to incorrect command in new tray icon implementation

4754991

Fix icon rendering on Linux

0821cb7

Handle left mouse click on the icon on Windows to show an app

92c964c

nazar-pc force-pushed the replace-tray-icon branch from 7d71043 to 92c964c Compare August 30, 2024 22:15

nazar-pc added 3 commits September 2, 2024 09:50

Make tray icon clicking toggle window visibility instead of just showing

5128459

Merge remote-tracking branch 'origin/main' into replace-tray-icon

b0a41ee

Rename second tray icon menu item to Quit

2d4a4b7

nazar-pc enabled auto-merge (squash) September 2, 2024 06:53

nazar-pc mentioned this pull request Sep 2, 2024

Add Spanish Language Support #294

Merged

nazar-pc merged commit 20f8ac3 into main Sep 2, 2024
8 checks passed

nazar-pc deleted the replace-tray-icon branch September 2, 2024 07:14

clostao added a commit that referenced this pull request Sep 12, 2024

fix: incorporate changes in #288

d1dce7a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `tray_icon` crate for better tray icon support #288

Use `tray_icon` crate for better tray icon support #288

dastansam commented Aug 12, 2024 •

edited

Loading

nazar-pc left a comment

nazar-pc Aug 12, 2024

nazar-pc Aug 12, 2024

dastansam Aug 14, 2024

nazar-pc Aug 15, 2024

nazar-pc Aug 12, 2024

dastansam Aug 14, 2024

nazar-pc Aug 15, 2024

nazar-pc left a comment

nazar-pc Aug 15, 2024

dastansam commented Aug 26, 2024

nazar-pc commented Aug 30, 2024

nazar-pc commented Aug 30, 2024

dastansam commented Sep 1, 2024 •

edited

Loading

nazar-pc commented Sep 1, 2024

Use tray_icon crate for better tray icon support #288

Use tray_icon crate for better tray icon support #288

Conversation

dastansam commented Aug 12, 2024 • edited Loading

nazar-pc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nazar-pc left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dastansam commented Aug 26, 2024

nazar-pc commented Aug 30, 2024

nazar-pc commented Aug 30, 2024

dastansam commented Sep 1, 2024 • edited Loading

nazar-pc commented Sep 1, 2024

Use `tray_icon` crate for better tray icon support #288

Use `tray_icon` crate for better tray icon support #288

dastansam commented Aug 12, 2024 •

edited

Loading

dastansam commented Sep 1, 2024 •

edited

Loading